Added support for complex variables #1467

andrewnester · 2024-06-03T16:46:42Z

Changes

Added support for complex variables

Now it's possible to add and use complex variables as shown below

bundle:
  name: complex-variables

resources:
  jobs:
    my_job:
      job_clusters:
        - job_cluster_key: key
          new_cluster: ${var.cluster}
      tasks:
      - task_key: test
        job_cluster_key: key

variables:
  cluster:
    description: "A cluster definition"
    type: complex
    default:
      spark_version: "13.2.x-scala2.11"
      node_type_id: "Standard_DS3_v2"
      num_workers: 2
      spark_conf:
        spark.speculation: true
        spark.databricks.delta.retentionDurationCheck.enabled: false

Fixes #1298

Support for complex variables
Allow variable overrides (with shortcut) in targets
Don't allow to provide complex variables via flag or env variable
Fail validation if complex value is used but not type: complex provided
Support using variables inside complex variables

Tests

Added unit tests

bundle/config/variable/variable.go

libs/dyn/convert/from_typed.go

libs/dyn/convert/normalize.go

libs/dyn/convert/to_typed_test.go

pietern

The dyn changes can use a bit more test coverage.

pietern · 2024-06-17T08:41:17Z

bundle/config/variable/variable.go

 )

 // An input variable for the bundle config
 type Variable struct {
+	// A type of the variable. This is used to validate the value of the variable
+	Type VariableType `json:"type,omitempty"`
+


Would we want to constrain the type of primitive types as well?

And otherwise, could we get away with not storing this at all?

We indeed could get away with not having this but there are few benefits of having this

It makes the intention of using complex variable explicit

It allows for visual editing tools to easily identify that the variable is of complex type and disable editing

It give the ability for future to extend the type with a known types, f.e. job_clusters and etc. I can imagine restricting the primitive types can be beneficial as well

libs/dyn/convert/from_typed.go

pietern · 2024-06-17T08:45:50Z

libs/dyn/convert/from_typed.go

+		if v.Kind() == reflect.Interface {
+			options = append(options, includeZeroValuedScalars)
+		}
+


This is to capture a variable value equal to 0, right?

Previously this would have been done through the non-nil pointer and dereferencing that.

Why do we need this?

It otherwise fails to assign dynamic complex value containing zero valued scalars

libs/dyn/convert/from_typed_test.go

libs/dyn/convert/normalize.go

libs/dyn/convert/from_typed.go

libs/dyn/convert/from_typed_test.go

bundle/tests/complex_variables_test.go

libs/dyn/convert/normalize.go

bundle/tests/variables/complex/databricks.yml

shreyas-goenka · 2024-06-17T13:45:40Z

We'll also need a PR similar to #1398 to update the JSON schema generated. To allow complex variable references.

Co-authored-by: shreyas-goenka <[email protected]>

bundle/tests/complex_variables_test.go

bundle/tests/variables/complex/databricks.yml

libs/dyn/convert/from_typed.go

libs/dyn/convert/normalize.go

bundle/config/mutator/resolve_variable_references.go

pietern · 2024-06-24T11:22:36Z

bundle/config/root.go

+				}
+
+				return variable, nil
+


Does the PR include test coverage for this, i.e. both ways of setting the variable, as well as defining a top level complex variable and then specifying a different type in the override?

Can you TAL at this?

bundle/tests/variables/complex/databricks.yml

libs/dyn/convert/from_typed.go

pietern · 2024-06-24T11:26:02Z

libs/dyn/convert/from_typed.go

+		if v.Kind() == reflect.Interface {
+			options = append(options, includeZeroValuedScalars)
+		}
+


Why do we need this?

libs/dyn/dynvar/ref.go

libs/dyn/path.go

pietern · 2024-06-25T14:52:59Z

bundle/config/mutator/resolve_variable_references.go

+						// Skip resolution if there is a skip function and it returns true.
+						if m.skipFn != nil && m.skipFn(v) {
+							return dyn.InvalidValue, dynvar.ErrSkipResolution
+						}


This can be moved outside of dynvar.Resolve.

pietern · 2024-06-25T14:53:25Z

bundle/config/root.go

+				}
+
+				return variable, nil
+


Can you TAL at this?

pietern

Remaining can be addressed async.

stevenayers-bge · 2024-06-26T07:48:59Z

Hi folks, great job on this PR. Will it allow you to access nested objects inside a variable, or will it just support array indexing?

Example:

bundle:
  name: complex-variables

resources:
  jobs:
    my_job:
      job_clusters:
        - job_cluster_key: key
          new_cluster: ${var.cluster}
      tasks:
      - task_key: "task ${var.cluster.foo}"
        job_cluster_key: key

variables:
  cluster:
    description: "A cluster definition"
    type: complex
    default:
      foo: bar
      spark_version: "13.2.x-scala2.11"
      node_type_id: "Standard_DS3_v2"
      num_workers: 2
      spark_conf:
        spark.speculation: true
        spark.databricks.delta.retentionDurationCheck.enabled: false

andrewnester · 2024-06-26T09:31:30Z

yes, you can access complex variable fields

CLI: * Add link to documentation for Homebrew installation to README ([#1505](#1505)). * Fix `databricks configure` to use `DATABRICKS_CONFIG_FILE` environment variable if exists as config file ([#1325](#1325)). Bundles: The Terraform upgrade to v1.48.0 includes a fix for library order not being respected. * Fix conditional in query in `default-sql` template ([#1479](#1479)). * Remove user credentials specified in the Git origin URL ([#1494](#1494)). * Serialize dynamic value for `bundle validate` output ([#1499](#1499)). * Override variables with lookup value even if values has default value set ([#1504](#1504)). * Pause quality monitors when "mode: development" is used ([#1481](#1481)). * Return `fs.ModeDir` for Git folders in the workspace ([#1521](#1521)). * Upgrade TF provider to 1.48.0 ([#1527](#1527)). * Added support for complex variables ([#1467](#1467)). Internal: * Add randIntn function ([#1475](#1475)). * Avoid multiple file tree traversals on bundle deploy ([#1493](#1493)). * Clean up unused code ([#1502](#1502)). * Use `dyn.InvalidValue` to indicate absence ([#1507](#1507)). * Add ApplyPythonMutator ([#1430](#1430)). * Set bool pointer to disable lock ([#1516](#1516)). * Allow the any type to be set to nil in `convert.FromTyped` ([#1518](#1518)). * Properly deal with nil values in `convert.FromTyped` ([#1511](#1511)). * Return `dyn.InvalidValue` instead of `dyn.NilValue` when errors happen ([#1514](#1514)). * PythonMutator: replace stdin/stdout with files ([#1512](#1512)). * Add context type and value to path rewriting ([#1525](#1525)). API Changes: * Added schedule CRUD commands to `databricks lakeview`. * Added subscription CRUD commands to `databricks lakeview`. * Added `databricks apps start` command. OpenAPI commit 7437dabb9dadee402c1fc060df4c1ce8cc5369f0 (2024-06-24) Dependency updates: * Bump golang.org/x/text from 0.15.0 to 0.16.0 ([#1482](#1482)). * Bump golang.org/x/term from 0.20.0 to 0.21.0 ([#1483](#1483)). * Bump golang.org/x/mod from 0.17.0 to 0.18.0 ([#1484](#1484)). * Bump golang.org/x/oauth2 from 0.20.0 to 0.21.0 ([#1485](#1485)). * Bump github.com/briandowns/spinner from 1.23.0 to 1.23.1 ([#1495](#1495)). * Bump github.com/spf13/cobra from 1.8.0 to 1.8.1 ([#1496](#1496)). * Bump github.com/databricks/databricks-sdk-go from 0.42.0 to 0.43.0 ([#1522](#1522)).

## Changes This PR: 1. Removes the custom added in https://github.com/databricks/cli/pull/1396/files for `variables.*.default`. It's no longer needed because with complex variables (#1467) `default` has a type of any. 2. Retains, and extends the override on `targets.*.variables.*`. Target override values can now be complex objects, not just primitive values. ## Tests Manually Before: Only primitive types were allowed. <img width="436" alt="Screenshot 2024-06-27 at 3 58 34 PM" src="https://github.com/databricks/cli/assets/88374338/de55be5c-9236-4ccc-b529-97ce7201b8e8"> After: An empty JSON schema is generated. All YAML values are acceptable. <img width="453" alt="Screenshot 2024-06-27 at 3 57 15 PM" src="https://github.com/databricks/cli/assets/88374338/7534b770-563f-4efc-b9c6-0af526dcc705">

Change variable struct to use any type instead of string

f8b286b

andrewnester requested a review from pietern June 3, 2024 16:46

pietern reviewed Jun 3, 2024

View reviewed changes

bundle/config/variable/variable.go Show resolved Hide resolved

libs/dyn/convert/from_typed.go Outdated Show resolved Hide resolved

libs/dyn/convert/normalize.go Outdated Show resolved Hide resolved

libs/dyn/convert/to_typed_test.go Outdated Show resolved Hide resolved

addressed feedback

f8b7d64

andrewnester requested a review from pietern June 4, 2024 10:03

added support for complex variables

26d1d5a

andrewnester marked this pull request as ready for review June 4, 2024 10:36

andrewnester changed the title ~~Change variable struct to use any type instead of string~~ Added support for complex variables Jun 4, 2024

This was referenced Jun 4, 2024

[FEATURE] DAB: Support for YAML Dictionary variables #1298

Closed

Support for Complex Types in Bundle Variables #1342

Closed

support overrides and do not allow setting via vars or envs

4503657

andrewnester requested a review from shreyas-goenka June 4, 2024 12:03

pietern reviewed Jun 17, 2024

View reviewed changes

added tests and fixes

aff20cf

andrewnester requested a review from pietern June 17, 2024 09:50

pietern reviewed Jun 17, 2024

View reviewed changes

libs/dyn/convert/normalize.go Outdated Show resolved Hide resolved

libs/dyn/convert/from_typed.go Outdated Show resolved Hide resolved

libs/dyn/convert/from_typed_test.go Show resolved Hide resolved

shreyas-goenka reviewed Jun 17, 2024

View reviewed changes

andrewnester and others added 3 commits June 18, 2024 13:37

Update libs/dyn/convert/normalize.go

f5d7127

Co-authored-by: shreyas-goenka <[email protected]>

Merge branch 'main' into feature/complex-variables

d8f50f2

fixes

f7eba5f

andrewnester requested review from shreyas-goenka and pietern June 19, 2024 13:49

pietern reviewed Jun 20, 2024

View reviewed changes

bundle/tests/complex_variables_test.go Show resolved Hide resolved

bundle/tests/variables/complex/databricks.yml Show resolved Hide resolved

libs/dyn/convert/from_typed.go Outdated Show resolved Hide resolved

andrewnester added 2 commits June 24, 2024 12:14

Merge branch 'main' into feature/complex-variables

443b338

allow using variables inside complex variables

ddd14eb

andrewnester requested a review from pietern June 24, 2024 10:30

pietern reviewed Jun 24, 2024

View reviewed changes

addressed feedback

0c4bf71

andrewnester requested a review from pietern June 24, 2024 13:02

pietern reviewed Jun 24, 2024

View reviewed changes

libs/dyn/dynvar/ref.go Outdated Show resolved Hide resolved

libs/dyn/path.go Outdated Show resolved Hide resolved

andrewnester added 2 commits June 24, 2024 15:23

fix test

44b17cd

allow nested indexed refs

95c0032

andrewnester requested a review from pietern June 24, 2024 15:12

andrewnester added 2 commits June 25, 2024 10:58

fixed test

59dba11

added test for referencing complex in complex error

327d05e

pietern reviewed Jun 25, 2024

View reviewed changes

pietern approved these changes Jun 25, 2024

View reviewed changes

added missing test

2e3c97b

andrewnester enabled auto-merge June 26, 2024 09:37

andrewnester added this pull request to the merge queue Jun 26, 2024

andrewnester removed this pull request from the merge queue due to a manual request Jun 26, 2024

andrewnester added this pull request to the merge queue Jun 26, 2024

Merged via the queue into main with commit 5f42791 Jun 26, 2024
5 checks passed

andrewnester deleted the feature/complex-variables branch June 26, 2024 10:31

pietern mentioned this pull request Jun 26, 2024

Release v0.222.0 #1529

Merged

shreyas-goenka mentioned this pull request Jun 27, 2024

Remove schema override for variable default value #1536

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for complex variables #1467

Added support for complex variables #1467

andrewnester commented Jun 3, 2024 •

edited by pietern

Loading

pietern left a comment

pietern Jun 17, 2024

andrewnester Jun 17, 2024

pietern Jun 17, 2024

pietern Jun 24, 2024

andrewnester Jun 24, 2024

shreyas-goenka commented Jun 17, 2024

pietern Jun 24, 2024

pietern Jun 25, 2024

pietern Jun 24, 2024

pietern Jun 25, 2024

pietern Jun 25, 2024

pietern left a comment

stevenayers-bge commented Jun 26, 2024

andrewnester commented Jun 26, 2024

Added support for complex variables #1467

Added support for complex variables #1467

Conversation

andrewnester commented Jun 3, 2024 • edited by pietern Loading

Changes

Tests

pietern left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shreyas-goenka commented Jun 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pietern left a comment

Choose a reason for hiding this comment

stevenayers-bge commented Jun 26, 2024

andrewnester commented Jun 26, 2024

andrewnester commented Jun 3, 2024 •

edited by pietern

Loading